Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP]Processing Jumpers and Loops, Plus Processing Wires inside the Table as Edges based on #251 #412

Draft
wants to merge 117 commits into
base: dev
Choose a base branch
from

Conversation

tobiasfalk
Copy link

Now, based on #251.
Thanks, @kvid, for your help with git(#251 (comment))
I will implement a fix for the problem that was discussed in #369 with @martinrieder and I will look into the implementation of the discussion in #286 by @formatc1702 and @martinrieder.

`edotor.net` does not seem to like leading underscores, which makes GraphViz debugging difficult.
no diff should ocurr as a result of the refactoring
to avoid diffs later when calling via CLI
for debugging purposes
kvid and others added 13 commits September 12, 2023 19:58
Allow absent "prefix" in group entries to simplify the code
Bug: 0x112233:0x445566 in YAML input didn't convert such colors
to #112233:#445566 and the strings where just passed as uppercase
to the .gv file. Hence Graphviz printed warnings about unknown
colors and used black as color instead.

Add test for int as string. Re-ordered if statements to give an
exception when a color has an unknown type.
wireviz#251 (comment)

No output changed for any examples/tutorial/tests input.
Removed from `Connector` class since it is already defined in the `Component` superclass.
@tobiasfalk tobiasfalk marked this pull request as draft July 20, 2024 14:00
@tobiasfalk tobiasfalk changed the title Processing Jumpers and Loops, Plus Processing Wires inside the Table as Edges based on #251 [WIP]Processing Jumpers and Loops, Plus Processing Wires inside the Table as Edges based on #251 Jul 20, 2024
@SnowMB
Copy link
Contributor

SnowMB commented Oct 13, 2024

Hey I tried your branch and ran into the following issues:

  • Examples do not build. Somehow in the html tables a pair of brackets is emited outside of a element
    • It is even present in the already commitid example files e.g. examples/ex01.gv:
    ...
  <td>
   <table border="0" cellborder="1" cellpadding="3" cellspacing="0">
    <tr>
     <td>GND</td>
     []
     <td port="p1r">1</td>
    </tr>
    <tr>
     <td>VCC</td>
     []
     <td port="p2r">2</td>
    </tr>
    <tr>
     <td>RX</td>
     []
     <td port="p3r">3</td>
    </tr>
    <tr>
     <td>TX</td>
     []
     <td port="p4r">4</td>
    </tr>
   </table>
  </td>
  ...
  • I traced this to this lines src/wireviz/wv_graphviz.py:293
 def gv_short_row_part(pin, connector) -> List:
     short_row = []# Td("ADA"), Td("DAD")
     for short, shPins in connector.shorts.items():
        if pin.index + 1 in shPins:
             short_row.append(Td("", port=f"p{pin.index+1}j"))
         else:
             short_row.append(Td(""))
-    return short_row
+    return short_row if len(short_row) > 0 else None
  • The old wire table still exists even though you now draw the edge ontop of the html table. There seems to be a small bug, that the inner html of the old wiretable defines columnspan=5 even though this table has only one column:
<tr>
     <td border="0" cellspacing="0" cellpadding="0" colspan="5" height="6" port="w1">
      <table border="0" cellborder="0" cellspacing="0">
       <tr>
        <td bgcolor="#FFFFFF" border="0" cellpadding="0" colspan="5" height="2"></td>
       </tr>
       <tr>
        <td bgcolor="#FFFFFF" border="0" cellpadding="0" colspan="5" height="2"></td>
       </tr>
       <tr>
        <td bgcolor="#FFFFFF" border="0" cellpadding="0" colspan="5" height="2"></td>
       </tr>
      </table>
     </td>
    </tr>
  • I guess the internal wire table can just be removed:
 def gv_wire_cell(wire: Union[WireClass, ShieldClass], colspan: int) -> Td:
-    if wire.color:
-        color_list = ["#000000"] + wire.color.html_padded_list + ["#000000"]
-    else:
-        color_list = ["#000000"]
-
-    wire_inner_rows = []
-    for j, bgcolor in enumerate(color_list[::-1]):
-        wire_inner_cell_attribs = {
-            "bgcolor": "#FFFFFF", # bgcolor if bgcolor != "" else "#000000", # TODO: More elegent solution for making black/whit space needed, since the wire is drawn as an actual edge
-            "border": 0,
-            "cellpadding": 0,
-            "colspan": colspan,
-            "height": 2,
-        }
-        wire_inner_rows.append(Tr(Td("", **wire_inner_cell_attribs)))
-    wire_inner_table = Table(wire_inner_rows, border=0, cellborder=0, cellspacing=0)
     wire_outer_cell_attribs = {
         "border": 0,
         "cellspacing": 0,
         "cellpadding": 0,
         "colspan": colspan,
-        "height": 2 * len(color_list),
+        "height": 6, # is 6 correct here? before it was 6 for single color wires
         "port": f"w{wire.index+1}",
     }
     # ports in GraphViz are 1-indexed for more natural maping to pin/wire numbers
-    wire_outer_cell = Td(wire_inner_table, **wire_outer_cell_attribs)
+    wire_outer_cell = Td(None, **wire_outer_cell_attribs)
 
     return wire_outer_cell

Should I create a merge request against your branch?

@tobiasfalk
Copy link
Author

@SnowMB Yes please, did not see that there is a problem with the html output.

@tobiasfalk
Copy link
Author

The old wire table still exists even though you now draw the edge ontop of the html table. There seems to be a small bug, that the inner html of the old wiretable defines columnspan=5 even though this table has only one column:

This was intentionally, since I was a bit leasy and it was the fastest🤦‍♂️

@ssweber
Copy link

ssweber commented Oct 17, 2024

Interested in this. (To anyone else you can see changes more clearly by comparing to refactor/big-refactor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants